Conversation
📝 WalkthroughWalkthroughAdds a new Distribution Status column to the analyses table UI with badge rendering and tooltip; centers the Run Status header; adjusts CSS for distribution-badge spacing and log card theming; updates unit tests to new column indices and introduces a reactive refresh mock for test data updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/analysis/logs/AnalysisLogCardContent.vue (1)
157-193:⚠️ Potential issue | 🟠 MajorAdd light-mode styling for
.log-scroll-panelto ensure readability in light mode.The
.log-scroll-panelonly has colors defined under.flame-dark(lines 183-186) but not for light mode. In light mode, it inherits the light background from.log-cardwhile keeping the hardcoded light text color (#f1f5f9), resulting in poor contrast.Add explicit light-mode styling:
Suggested fix
.flame-light .log-scroll-panel { background: var(--p-slate-50); color: var(--p-slate-900); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/logs/AnalysisLogCardContent.vue` around lines 157 - 193, The .log-scroll-panel lacks explicit light-mode colors causing poor contrast in light themes; add a .flame-light .log-scroll-panel rule that sets an appropriate light background and dark text (e.g., use --p-slate-50 for background and --p-slate-900 for color) so .log-scroll-panel has explicit styling in both .flame-dark and .flame-light themes and remains readable.
🧹 Nitpick comments (1)
app/components/analysis/AnalysesTable.vue (1)
649-665: Consider extracting repeated badge markup into a shared component or helper.The Distribution Status badge structure (lines 649-665) is nearly identical to the Data Store badge (lines 743-756). Both use Badge with check/times icons, tooltips, and similar severity logic.
This could be extracted to reduce duplication, though it's minor given the scope.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysesTable.vue` around lines 649 - 665, Extract the duplicated Badge markup into a small reusable component (e.g., StatusBadge) and replace both occurrences in AnalysesTable.vue: create a StatusBadge component that accepts a boolean/condition prop (e.g., "active" or "condition"), active/inactive tooltip text and optional severity/icon props (defaults: active -> severity="success" + pi-check + "Image available", inactive -> severity="danger" + pi-times + "Image unavailable"), render the same wrapper class ("distribution-badge") and Badge element with v-tooltip and icon based on the condition, then update the distribution_status rendering (where you check data.analysis.distribution_status === 'executed') and the Data Store badge (where you check data.store or similar) to use <StatusBadge :condition="..." :active-tooltip="..." :inactive-tooltip="..."/> so both places reuse the component and preserve existing classes and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 634-667: The Column is marked sortable but uses
field="distribution" while the template reads data.analysis.distribution_status,
so sorting operates on undefined; fix by making the Column's field match the
actual data path (use "analysis.distribution_status" or the exact property used)
or supply a proper sortField/field extractor for the Column component to
reference data.analysis.distribution_status (update the Column declaration where
field="distribution" and ensure :sortable remains true so sorting targets the
real distribution_status value).
---
Outside diff comments:
In `@app/components/analysis/logs/AnalysisLogCardContent.vue`:
- Around line 157-193: The .log-scroll-panel lacks explicit light-mode colors
causing poor contrast in light themes; add a .flame-light .log-scroll-panel rule
that sets an appropriate light background and dark text (e.g., use --p-slate-50
for background and --p-slate-900 for color) so .log-scroll-panel has explicit
styling in both .flame-dark and .flame-light themes and remains readable.
---
Nitpick comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 649-665: Extract the duplicated Badge markup into a small reusable
component (e.g., StatusBadge) and replace both occurrences in AnalysesTable.vue:
create a StatusBadge component that accepts a boolean/condition prop (e.g.,
"active" or "condition"), active/inactive tooltip text and optional
severity/icon props (defaults: active -> severity="success" + pi-check + "Image
available", inactive -> severity="danger" + pi-times + "Image unavailable"),
render the same wrapper class ("distribution-badge") and Badge element with
v-tooltip and icon based on the condition, then update the distribution_status
rendering (where you check data.analysis.distribution_status === 'executed') and
the Data Store badge (where you check data.store or similar) to use <StatusBadge
:condition="..." :active-tooltip="..." :inactive-tooltip="..."/> so both places
reuse the component and preserve existing classes and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63a10d85-0d48-4593-926b-f9ff1f7a208b
📒 Files selected for processing (3)
app/components/analysis/AnalysesTable.vueapp/components/analysis/logs/AnalysisLogCardContent.vuetest/components/analysis/AnalysisTable.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/analysis/AnalysesTable.vue (1)
882-884: Use layout centering instead of a hardcoded left margin.
margin-left: 4remis fragile across viewport/table changes. Center via flex to keep badge placement stable.♻️ CSS refactor
.distribution-badge { - margin-left: 4rem; + display: flex; + justify-content: center; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/analysis/AnalysesTable.vue` around lines 882 - 884, Replace the fragile hardcoded offset on .distribution-badge with flex-based centering: remove margin-left: 4rem and instead center the badge using flexbox (e.g., make the badge's container a flex row and use justify-content: center and align-items: center, or set .distribution-badge to display:flex with justify-content:center/align-items:center if it is the direct container). Update the CSS rule for .distribution-badge (and its parent container if needed) to use these flex properties so the badge stays centered responsively instead of relying on a fixed left margin.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 650-665: The UI currently collapses every non-executed
distribution_status into a single "unavailable" state; update the rendering in
AnalysesTable.vue to map each possible distribution_status value (use the known
values from Api.ts around distribution_status) to its own Badge severity, icon
and tooltip instead of using a binary executed vs else branch—replace the
v-if/v-else that only checks distribution_status === 'executed' with a small
status-to-metadata mapping (status -> {severity, icon, tooltip}) and render one
Badge using that mapping (keep using v-tooltip and the pi- icons but choose
pi-check, pi-spinner, pi-times, etc. appropriately) so in-progress, stopped,
failed, executed and other statuses show distinct visuals and tooltips.
---
Nitpick comments:
In `@app/components/analysis/AnalysesTable.vue`:
- Around line 882-884: Replace the fragile hardcoded offset on
.distribution-badge with flex-based centering: remove margin-left: 4rem and
instead center the badge using flexbox (e.g., make the badge's container a flex
row and use justify-content: center and align-items: center, or set
.distribution-badge to display:flex with
justify-content:center/align-items:center if it is the direct container). Update
the CSS rule for .distribution-badge (and its parent container if needed) to use
these flex properties so the badge stays centered responsively instead of
relying on a fixed left margin.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0df0e4eb-5680-440b-9be1-012dd81fc8fb
📒 Files selected for processing (1)
app/components/analysis/AnalysesTable.vue
And fix log colors
Summary by CodeRabbit
New Features
Style
Tests